Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for PHP 8 #539

Closed
wants to merge 3 commits into from
Closed

Support for PHP 8 #539

wants to merge 3 commits into from

Conversation

Korben00
Copy link

@Korben00 Korben00 commented Mar 6, 2024

Description

This pull request includes updates to transition the project's PHP support to version 8.0. These changes are necessary to ensure the project stays compatible with the latest PHP version, providing improved performance, security, and newer language features. The update involves modifications to composer.json, composer.lock, and readme.txt files, alongside code adjustments in src/Integration/DefaultLogger.php to align with PHP 8 standards.

Changes

1. composer.json and composer.lock:

  • Updated PHP requirement to "php": "8.0".
  • Upgraded dependencies to their PHP 8 compatible versions. This includes major version upgrades for packages such as symfony/yaml from ^2.6 to ^5.3, phpunit/phpunit from * to ^9.3, and psr/log from ^1.0 to ^3.0, among others.
  • Adjusted platform config setting to PHP 8.0, ensuring consistency across development and production environments.

2. src/Integration/DefaultLogger.php:

  • Implemented type declarations and return type void for methods log and debug to comply with the PHP 8 standards.
  • Replaced array notation with [] for default parameter values and function return types.
  • Introduced Stringable|string for $message parameter type hinting, leveraging PHP 8's union types and promoting more flexible logging capabilities.

3. readme.txt:

  • Updated the "Requires PHP" version to 8.0 to inform users and contributors about the new PHP version requirement.

@LeoColomb
Copy link

.github/workflows/php.yml Outdated Show resolved Hide resolved
@LeoColomb
Copy link

Mentioning @aseure, just in case 😊

@Korben00
Copy link
Author

Any time to review ?

@aseure
Copy link
Member

aseure commented Mar 25, 2024

Hey @Korben00 and @LeoColomb.

Thank you very much for the contribution and your time here.

While we would like to better support PHP 8 in the future, an important user base of this plugin is using PHP 7 installation. Even though this version is indeed outdated, and many Wordpress users would gladly upgrade to PHP 8 (or already did), a non-negligible amount of users will still be using PHP 7 (either because they don't know how to upgrade or because their hosted Wordpress will not upgrade). Due to this and the fact that most users have the plugin auto-upgrade setting enable, I'm concerned about breaking their installations.

I'll let @jacobbednarz comment on this, who perhaps have more insights than I have on this. cc @william-woodhead as well.

@lkraav
Copy link

lkraav commented Mar 25, 2024

Indeed, standard practice still today is to keep at least PHP 7.4 in play.

@LeoColomb
Copy link

@aseure @lkraav Thanks for your replies.
While I'm personally not a big fan of standard practices actually specific to WordPress, I understand the reasoning. 😊

Thus, we've just submitted another PR (#541) which does not touch the platform version but allows working around the compatibility/conflict issue with the dependencies.

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Sep 22, 2024
@github-actions github-actions bot closed this Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants